Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added Functions for EigenValues and EigenVectors #14

Closed
wants to merge 2 commits into from

Conversation

Shlufi
Copy link

@Shlufi Shlufi commented Apr 21, 2015

Added two functions (one for double & the other for float) that calculate the eigenvalues and eigenvectors for a given Matrix. I'm not sure how to best organize the functions and their return statements - maybe having separate functions for retrieving eigenvectors and eigenvalues would be appropriate.

Ben Robbins and others added 2 commits April 20, 2015 23:16
mattt added a commit that referenced this pull request Apr 23, 2015
Refactoring and reformatting
@mattt
Copy link
Collaborator

mattt commented Apr 23, 2015

Thanks for your PR, @Shlufi, but it doesn't compile.

I was able to make the necessary changes with 331f7bc, which is available on the matrix-eigendecomposition branch. Could you please add some unit tests to ensure this is working as intended?

@Shlufi
Copy link
Author

Shlufi commented Apr 24, 2015

Sure I should be able to get to that at some point this week.

@gaming-hacker
Copy link

any updates?

@loufranco
Copy link
Contributor

loufranco commented Feb 7, 2019

@mattt I made a fork, merged master and resolved conflicts on the matrix-eigendecomposition branch.

Then, I added a commit to fix bugs and added tests. I am ready to PR, but would like some guidance on a few things (if you prefer to just comment on the PR, let me know and I'll make it)

  1. _dgeev has 3 returns, the eigenValues and the left and right eigen vectors -- I represented that as a 3-tuple, but got a lint warning. I could make (a) a struct for just this return or (b) follow numpy and just return the right eigenvector (not the left) -- or (c) could suppress the warning for this function.

  2. Any of these returns could have complex results (Real + Imaginary) -- so I can't use Matrix<> or [Double]-- instead I just used [(Double, Double)] for vectors and [[(Double, Double)]] for complex matrices. Does that sound ok? There isn't a Complex in the Surge and I didn't want to just add that.

  3. I didn't see any standard way to handle errors in the underlying calls -- most things just assert on error (e.g. for when a matrix doesn't have a determinant). So, I did that too.

@loufranco
Copy link
Contributor

I made this PR for discussion: https://github.com/mattt/Surge/pull/95

It works, but has the issues outlined above

@alejandro-isaza
Copy link
Collaborator

I replied to 1 in the PR. For 2 that is ok for now but we should add a Complex type. We should move it from Upsurge. For 3 let's throw errors where appropriate. Thanks.

@regexident
Copy link
Collaborator

I'm closing this due to inactivity. 💀

If you still feel that Surge should get this functionality feel free to open an updated PR for it. 🙂

@regexident regexident closed this Nov 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants